-
Notifications
You must be signed in to change notification settings - Fork 3.3k
feat(elasticsearch): support for composable index templates #15089
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(elasticsearch): support for composable index templates #15089
Conversation
Co-authored-by: sergio.gomez <[email protected]>
|
Cursor Agent can help with this pull request. Just |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (3.33%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage. 📢 Thoughts on this report? Let us know! |
Co-authored-by: sergio.gomez <[email protected]>
Co-authored-by: sergio.gomez <[email protected]>
Co-authored-by: sergio.gomez <[email protected]>
…te-ingestion-issue-5643
| ): | ||
| yield mcp.as_workunit() | ||
| except Exception as e: | ||
| logger.debug(f"Unable to fetch composable index templates: {e}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it ok to log this as debug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitely not an error as composable index templates is sort of optional
I can set warning
| custom_properties["num_replicas"] = num_replicas | ||
| # 4. Construct and emit properties | ||
| if is_index: | ||
| custom_properties: Dict[str, str] = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this the same as _extract_template_custom_properties?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar, but not the same.
In the case of template, it needs to handle different depending on is_composable
…te-ingestion-issue-5643
…te-ingestion-issue-5643
Fix(elasticsearch): Ingest both legacy and composable index templates
Problem:
The Elasticsearch/OpenSearch source connector previously only ingested legacy index templates (using the
get_template()API). Modern OpenSearch (v2.17) and Elasticsearch (7.8+) also support composable index templates, which are accessed via a different API (get_index_template()) and have a distinct data structure. This led to incomplete index template ingestion, where only legacy templates were discovered, or none if only composable templates existed.Solution:
This PR updates the Elasticsearch source to support both types of index templates:
get_workunits_internalmethod now attempts to fetch both legacy and composable index templates._extract_mcpsmethod has been enhanced to correctly parse the metadata (mappings, settings, aliases) from both legacy templates (root-level fields) and composable templates (fields nested undertemplate).Impact:
This bug fix ensures that all index templates (both legacy and composable) are now correctly ingested from OpenSearch and Elasticsearch clusters, providing comprehensive metadata coverage. The changes are backward compatible with older clusters that only use legacy templates.
Related Issue:
CUS-6603
Slack Thread